Skip to content

Feat add status cli command#171

Open
Ismael wants to merge 9 commits into
ory:mainfrom
Ismael:feat-status
Open

Feat add status cli command#171
Ismael wants to merge 9 commits into
ory:mainfrom
Ismael:feat-status

Conversation

@Ismael
Copy link
Copy Markdown

@Ismael Ismael commented May 30, 2026

Adds a lumen status [path] CLI command that reports embedding-service health and index freshness for a project - the same two checks the doctor skill runs against the MCP server (health_check + index_status), but standalone and
script-friendly.

$ lumen status
Embedding service: OK (ollama, http://localhost:11434, jina-embeddings-v2-base-code)
Index: /home/me/proj
  Files: 164 | Indexed: 164 | Chunks: 1698 | Model: jina-embeddings-v2-base-code
  Last indexed: 2026-05-30T17:33:53Z | Stale: no

Behavior

  • Path: optional arg, defaults to cwd. Normalized to the git root / ancestor index via the shared resolveIndexRoot helper, so it reports DB that search uses.
  • Embedding servers: every configured server is probed concurrently and reported on its own line.
  • Index: read-only — status never indexes, seeds, or creates a DB. A missing index DB is reported as "not indexed".
  • Exit code: 0 only when at least one server is reachable and the index exists and it is fresh; otherwise 1. The "at least one server" rule matches lumen's failover model: the service is usable as long as one backend is up.

Notable design points

  • No side effects: the DB path is os.Stat-checked before opening, so inspecting an un-indexed project never creates an empty database. Covered by TestRunStatusMissingIndexNoSideEffect.
  • DRY: the embedding-service probe was extracted from the MCP handleHealthCheck handler into a shared probeEmbeddingService, and path normalization reuses the existing resolveIndexRoot helper rather than
    re-rolling the git-root→ancestor logic.

Summary by CodeRabbit

  • New Features

    • Added lumen status to report embedding service reachability and index freshness with actionable exit codes.
    • Automatic index seeding from sibling worktrees, performed under the index lock so only one instance seeds.
    • Coordinated foreground/background indexer behavior with a timed wait to prevent concurrent creation or clobbering of indexes.
  • Tests

    • New tests for status reporting, seeding from donors, create-wait coordination, seed-race avoidance, and embedding-service reachability.

Review Change Stack

Ismael and others added 7 commits May 30, 2026 11:27
The lumen index command (the background indexer spawned at SessionStart)
created the DB and re-indexed every file from scratch instead of copying
an existing sibling-worktree index — only the MCP getOrCreate seeded, and
it lost the race to create the DB. runIndexer now seeds from a donor,
under the index lock it already holds, before indexing.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
getOrCreate seeded and created a new DB without the index flock that
lumen index holds, so both could run SeedFromDonor against the same fresh
worktree DB concurrently and corrupt it. getOrCreate now takes the same
lock to seed; when a peer holds it, it waits briefly for the peer to
publish the DB instead of creating an empty one that clobbers the seed.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Probe every configured server concurrently and print one line per
server. With failover, the service counts as healthy when at least one
server is reachable, so exit is non-zero only when all are unreachable.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
collectStatus duplicated the git-root → ancestor-index normalization
that resolveIndexRoot already encapsulates (and which search uses).
Reuse the shared helper instead, removing the duplication, its drift
risk, and gaining the symlink resolution status was silently missing.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 30, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 8d06fc21-633b-486e-aafa-1d22a7a7d851

📥 Commits

Reviewing files that changed from the base of the PR and between 2f9ba8a and 7e541ca.

📒 Files selected for processing (1)
  • cmd/status.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • cmd/status.go

📝 Walkthrough

Walkthrough

Adds donor-based seeding of missing worktree index DBs, a timed create-wait with lock-serialized DB creation to avoid seed races, and a new lumen status command that reports embedding-service reachability and index freshness.

Changes

Index seeding, status diagnostics, and creation coordination

Layer / File(s) Summary
Donor-based index seeding initialization
cmd/seed.go, cmd/seed_test.go, cmd/index.go
New seedFromDonorIfNew checks DB existence and seeds missing DBs from a sibling donor via config.FindDonorIndex and index.SeedFromDonor, with test verifying metadata inheritance. Integrated into runIndexer under the index lock before initialization.
Index creation coordination: timeouts, locking, and probe helpers
cmd/stdio.go, cmd/stdio_probe_test.go, cmd/stdio_seedrace_test.go
Adds create-wait constants and indexerCache timeout override; implements lock-serialized DB creation (acquire lock, seed if owner, wait if peer holds lock); extracts seedFromDonor and waitForDB helpers; refactors embedding-service probe into probeEmbeddingService. Includes regression test preventing seed-while-lock-held and probe tests for multiple scenarios.
Status command: types, evaluation, formatting, and execution
cmd/status.go, cmd/status_test.go
New lumen status CLI command reports embedding server reachability and project index freshness. Defines serverStatus and statusResult, implements anyServerReachable and statusExitCode, formats output with defaults for unindexed/stale states, registers CLI flags, and implements runStatus/collectStatus to probe servers concurrently, check DB existence, read index metadata, and compute staleness. Tests cover exit code logic, output formatting, and side-effect isolation.

Sequence Diagram

sequenceDiagram
  participant Foreground as Foreground Indexer
  participant Lock as Index Lock
  participant DB as Index DB File
  participant Donor as Donor Worktree

  Foreground->>Lock: TryAcquire (DB lock)
  alt Lock acquired
    Foreground->>DB: Stat (check exists)
    alt DB missing
      Foreground->>Donor: FindDonorIndex / SeedFromDonor
      Donor->>DB: Create seeded DB file
    end
  else Lock held by peer
    Foreground->>DB: Poll for file (createWaitTimeout)
    DB->>Foreground: File appears (seeded by peer)
  end
  Foreground->>Foreground: Open and use DB
Loading

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title refers to adding a status CLI command, which is a real and significant part of the changeset, but the PR involves multiple coordinated changes including seeding logic, background indexer integration, health check refactoring, and comprehensive testing.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
cmd/stdio.go (1)

520-542: ⚡ Quick win

Defer lock release for consistency and panic safety.

The lock is released immediately without defer, but cmd/index.go:252 defers lock.Release() in a similar pattern. While business logic should never panic per guidelines, deferring the release ensures cleanup even if seedFromDonor encounters an unexpected panic, and makes the code consistent with the existing locking pattern.

♻️ Proposed fix
 	lockPath := indexlock.LockPathForDB(dbPath)
 	lk, lockErr := indexlock.TryAcquire(lockPath)
 	switch {
 	case lockErr == nil && lk != nil:
+		defer lk.Release()
 		// We own creation. Re-check under the lock — a peer may have created
 		// the DB between our stat above and acquiring the lock.
 		if _, st := os.Stat(dbPath); os.IsNotExist(st) {
 			seedWarning = ic.seedFromDonor(effectiveRoot, modelName, dbPath)
 		}
-		lk.Release()
 	default:

As per coding guidelines: Always defer resource cleanup for database and file handles using defer Close() (lock resources follow the same pattern).

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@cmd/stdio.go` around lines 520 - 542, After successfully acquiring the index
lock (lockPath via indexlock.TryAcquire returning lk), immediately defer
lk.Release() to guarantee the lock is released on panic or early return; remove
the explicit lk.Release() call later in the same block so you don't release
twice. Keep the re-check of os.Stat(dbPath) and the seed call (ic.seedFromDonor)
unchanged, and ensure the default branch still calls ic.waitForDB(dbPath). This
mirrors the pattern used around the same lock in the codebase (see the existing
defer usage) and ensures deterministic cleanup.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@cmd/stdio.go`:
- Around line 520-542: After successfully acquiring the index lock (lockPath via
indexlock.TryAcquire returning lk), immediately defer lk.Release() to guarantee
the lock is released on panic or early return; remove the explicit lk.Release()
call later in the same block so you don't release twice. Keep the re-check of
os.Stat(dbPath) and the seed call (ic.seedFromDonor) unchanged, and ensure the
default branch still calls ic.waitForDB(dbPath). This mirrors the pattern used
around the same lock in the codebase (see the existing defer usage) and ensures
deterministic cleanup.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: c38dd292-5087-4eaa-ba0e-c98bdccaa962

📥 Commits

Reviewing files that changed from the base of the PR and between d0dee0e and f0def81.

📒 Files selected for processing (8)
  • cmd/index.go
  • cmd/seed.go
  • cmd/seed_test.go
  • cmd/status.go
  • cmd/status_test.go
  • cmd/stdio.go
  • cmd/stdio_probe_test.go
  • cmd/stdio_seedrace_test.go

Ismael and others added 2 commits May 30, 2026 15:44
The seed branch released the index lock with an explicit lk.Release(),
which leaks the lock if seedFromDonor panics or returns early. Wrap the
seed block in an IIFE with defer lk.Release() so cleanup is deterministic
while keeping the lock scoped to the seed block.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
errcheck flagged the unchecked write in the status command. Propagate
the write error instead of ignoring it.

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@Ismael
Copy link
Copy Markdown
Author

Ismael commented May 30, 2026

Note that this branch includes the commits from #170 because that extracted one common function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant